- 
                Notifications
    You must be signed in to change notification settings 
- Fork 35
Remove continue-on-error Windows jobs ci #313
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove continue-on-error Windows jobs ci #313
Conversation
| Codecov ReportAttention: Patch coverage is  
 
 Additional details and impacted files@@            Coverage Diff             @@
##             main     #313      +/-   ##
==========================================
+ Coverage   74.27%   74.29%   +0.01%     
==========================================
  Files           8        8              
  Lines        3188     3190       +2     
==========================================
+ Hits         2368     2370       +2     
  Misses        820      820              
 
 | 
aec490f    to
    c879a57      
    Compare
  
    | @vgvassilev looking at the output of the ci here https://github.com/compiler-research/CppInterOp/actions/runs/10288514553/job/28502994037 it appears that only a few symbols are left to be exported to get std::cout and std::cerr to work in xeus-cpp on Windows, but I can't work out where they need to be exported, since both CppInterOp and xeus-cpp are exporting the symbols it is complaining about. Any idea what these symbols need to be exported? | 
f640d31    to
    e507f53      
    Compare
  
    | clang-tidy review says "All clean, LGTM! 👍" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
24ab8b9    to
    de94995      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
| clang-tidy review says "All clean, LGTM! 👍" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
| clang-tidy review says "All clean, LGTM! 👍" | 
    
      
        1 similar comment
      
    
  
    | clang-tidy review says "All clean, LGTM! 👍" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
| clang-tidy review says "All clean, LGTM! 👍" | 
    
      
        3 similar comments
      
    
  
    | clang-tidy review says "All clean, LGTM! 👍" | 
| clang-tidy review says "All clean, LGTM! 👍" | 
| clang-tidy review says "All clean, LGTM! 👍" | 
039bf03    to
    8b248a3      
    Compare
  
    | clang-tidy review says "All clean, LGTM! 👍" | 
    
      
        4 similar comments
      
    
  
    | clang-tidy review says "All clean, LGTM! 👍" | 
| clang-tidy review says "All clean, LGTM! 👍" | 
| clang-tidy review says "All clean, LGTM! 👍" | 
| clang-tidy review says "All clean, LGTM! 👍" | 
bfc0f56    to
    14b0a6a      
    Compare
  
    | clang-tidy review says "All clean, LGTM! 👍" | 
| clang-tidy review says "All clean, LGTM! 👍" | 
a7093c3    to
    a256714      
    Compare
  
    | clang-tidy review says "All clean, LGTM! 👍" | 
8400eb5    to
    9b86ee2      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm modulo the comment.
| @vgvassilev getenv_s is not being recognised on Unix systems. Any ideas why? | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
0f1a770    to
    da0902c      
    Compare
  
    da0902c    to
    0f4f348      
    Compare
  
    | 
 looks like  X-ref: https://stackoverflow.com/questions/372980/do-you-use-the-tr-24731-safe-functions/373911#373911 | 
The objective of this PR is to add to the ci a non wasm build of xeus-cpp which runs its various tests. This should allow debugging of the missing symbols needed to make Windows work (discussion here compiler-research/xeus-cpp#144).